Skip to content

feat(feishu): persist imported document images - #3033

Merged
qin-ctx merged 2 commits into
volcengine:mainfrom
dfwgj:wgj-feishu-05-inline-images
Jul 7, 2026
Merged

feat(feishu): persist imported document images#3033
qin-ctx merged 2 commits into
volcengine:mainfrom
dfwgj:wgj-feishu-05-inline-images

Conversation

@dfwgj

@dfwgj dfwgj commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

Downloads and persists inline Feishu document images during import. Feishu image references such as feishu://image/{file_token} are resolved into local temporary image files before Markdown parsing, so the existing Markdown image ingestion flow can store them in VikingFS and rewrite references to stable viking:// image URIs.

Related Issue

Fixes #3030

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Added Feishu image token detection for feishu://image/{file_token} Markdown references.
  • Downloaded Feishu image bytes into a temporary import directory beside document.md.
  • Rewrote Feishu image references to local relative paths so MarkdownParser can ingest them normally.
  • Added cleanup support for temporary directories through _cleanup_path.
  • Added feishu.download_images configuration support.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Test coverage added/updated:

  • tests/parse/test_feishu_accessor.py

Manual validation:

  • Imported a Feishu document containing inline images.
  • Verified image files are written next to the generated Markdown.
  • Verified Markdown image references are rewritten to local relative paths before ingestion.
  • Verified imported images can be previewed from VikingFS.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

  • original file reference
image - OpenViking Studio preview image - L1-level description image

Additional Notes

No public API change is required. Image downloading is controlled by the Feishu parser configuration via download_images.

@qin-ctx

qin-ctx commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Please add Feishu-specific verification details:

  1. A test screenshot of the parsed/imported result, showing that the image from the Feishu document is rendered correctly after import.
  2. A concrete example from a Feishu document after inserting an image, including the generated source Markdown around the image reference, e.g. the exact feishu://image/{file_token} reference, and the rewritten image path/URI after import if available.

This will make it easier to verify that the Feishu image-token parsing and rewrite flow covers the real exported document format.

@qin-ctx qin-ctx self-assigned this Jul 7, 2026
Download Feishu image tokens into local import temp trees so Markdown image references can be ingested alongside the document.

Co-authored-by: Cursor <cursoragent@cursor.com>
@dfwgj
dfwgj force-pushed the wgj-feishu-05-inline-images branch from 866b2a1 to 30d3dda Compare July 7, 2026 05:23
@dfwgj

dfwgj commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@qin-ctx Hi, I've uploaded the screenshot to the Screenshots (if applicable) section above.

Feishu-image ingestion deliberately reuses the existing local-file image pipeline. The only Feishu-specific work happens inside the accessor: it downloads each feishu://image/{file_token} reference and lays the bytes out on disk exactly like a plain local Markdown document (a document.md plus a sibling images/ folder with relative references). From that point on, _ingest_local_images and rewrite_image_uris run identically for both sources — no Feishu-specific branch downstream.

Path comparison

Stage Plain local file Feishu document image
Source marker relative path already in the file, e.g. images/pic.png ![alt](feishu://image/{file_token}) produced by _handle_image (converter.py:571)
Get the bytes already on disk — no download _download_image(file_token) via Feishu Drive medias/{token}/download (feishu_accessor.py:179)
Materialize to a temp tree in place (the original folder) access writes temp_dir/document.md + temp_dir/images/{filename} and rewrites the ref to images/{filename} (feishu_accessor.py:102-110, _resolve_image_refs :292)
base_dir for resolution Markdown file's parent (markdown.py:215) temp_dir (markdown.py:215) — same mechanism
Ingest _ingest_local_images (markdown.py:550) same code, reused
Validation _is_valid_image (markdown.py:756) same code, reused
Sidecar .image_mappings.json (markdown.py:669) same
Rewrite rewrite_image_uris (image_rewrite.py:197) same
Final form viking://.../pic.png viking://.../{filename}

Concrete before / after for a Feishu document

1. Converter output (Markdown as produced from the Feishu doc):

![screenshot](feishu://image/boxcnXXXXXXXXXXXXXXXXXX)

2. After the accessor downloads and rewrites — temp_dir layout handed to the parser:

temp_dir/
  document.md          # ![screenshot](images/boxcnXXXXXXXXXXXXXXXXXX.png)
  images/
    boxcnXXXXXXXXXXXXXXXXXX.png

3. After _ingest_local_images + rewrite_image_uris (final, committed form):

![screenshot](viking://resources/.../boxcnXXXXXXXXXXXXXXXXXX.png)

This is byte-for-byte the same shape a plain local doc.md + ./images/pic.png ends up in — the only difference is where the bytes came from (Feishu Drive API vs. already on disk).

Notes

  • Download failure is non-fatal. If _download_image returns None (e.g. missing scope docs:document.media:download), the token is skipped and its original feishu://image/... reference is left in place; the rest of the import still succeeds (feishu_accessor.py:329-330).

@qin-ctx qin-ctx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found two blocking issues in the Feishu image import path: the new media downloads run synchronously inside the async accessor, and downloaded images are always named as .png even when the bytes may be JPEG/WebP/GIF. Please address these before merge.

Comment thread openviking/parse/accessors/feishu_accessor.py Outdated
Comment thread openviking/parse/accessors/feishu_accessor.py Outdated
@dfwgj
dfwgj force-pushed the wgj-feishu-05-inline-images branch from 4d30945 to 30d3dda Compare July 7, 2026 08:52

@qin-ctx qin-ctx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one blocking issue in the Feishu image import path: user-token imports can read the document body but the new media download request only allows tenant-token auth, so inline images are not persisted for the same import mode.

Comment thread openviking/parse/accessors/feishu_accessor.py Outdated
Address review feedback on the inline-image import path:

- Run the synchronous lark-oapi media download via asyncio.to_thread so a
  slow Feishu request no longer blocks unrelated async work on the event loop,
  matching the existing _fetch_document() pattern.
- Infer the image file extension from the downloaded bytes (byte-magic
  sniffing) and fall back to the response Content-Type via the existing
  mime_types.get_preferred_extension helper, instead of hardcoding .png. This
  stops JPEG/WebP/GIF bytes from being mislabeled as PNG to downstream
  consumers (e.g. the data:image/... URI built during multimodal vectorization).
- Advertise AccessTokenType.USER on the media download request when a user
  access token is supplied, so lark-oapi actually injects it. Previously the
  request only allowed TENANT, so user-token imports read the document body
  but silently dropped every image.

Co-authored-by: Cursor <cursoragent@cursor.com>
@dfwgj
dfwgj force-pushed the wgj-feishu-05-inline-images branch from 1c762b1 to 30d3dda Compare July 7, 2026 09:19
@dfwgj

dfwgj commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@qin-ctx Thanks for the thorough review.!All three blocking issues are addressed in the latest commit:

  1. Sync download on the async path — the media download now runs via asyncio.to_thread, following the existing _fetch_document() pattern, so it no longer blocks the event loop.
  2. Hardcoded .png — the extension is now sniffed from the image bytes (byte magic), falling back to the response Content-Type via the existing mime_types.get_preferred_extension helper, so JPEG/WebP/GIF are no longer mislabeled as PNG downstream.
image
  1. Tenant-only media auth — the download request now advertises AccessTokenType.USER when a user access token is supplied (confirmed against lark_oapi.core.token.auth.verify), so user-token imports keep their images instead of silently dropping them.

Added unit tests covering extension detection (Content-Type + byte magic), the event-loop offload, and both the tenant- and user-token request paths.

One note on the byte-magic sniffing: I'm aware this logic is conceptually duplicated across the codebase — http_accessor._detect_from_magic_bytes and the three VLM backends' _detect_image_format all sniff image magic bytes. I deliberately kept a small local copy here rather than reuse them, because none is directly reusable: http_accessor returns (URLType, ext) tuples tied to its URL-classification flow, and the VLM helpers return format-name strings for API payloads — neither maps cleanly to "bytes → file extension." Factoring these into a shared helper (e.g. mime_types.sniff_image_extension) and migrating all call sites feels out of scope for this PR and touches unrelated modules (VLM backends). I'd suggest doing that consolidation in a dedicated follow-up PR. Happy to file it if you agree.

@qin-ctx
qin-ctx merged commit 07aa9dc into volcengine:main Jul 7, 2026
7 of 8 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Feature]: Download and persist inline Feishu document images during import

2 participants